I think we should replace the callbacks in favor of a single method that handles view actions. A sealed class representing all the view actions can then just be appended to.

    sealed interface ViewAction {
        data class OnBrandChoiceChanged(val cardBrandChoice: CardBrandChoice) : ViewAction
        data class OnCardDetailsChanged(val cardUpdateParams: CardUpdateParams?) : ViewAction
    }

• fun onBrandChoiceChanged(cardBrandChoice: CardBrandChoice) is a function meant to be called from the UI when a new brand is selected.
• val onBrandChoiceChanged: BrandChoiceCallback is meant for analytics purposes. It's fired when the selected brand in the function above is different from the original. The brandChoiceChange function parameter is sent downstream from [Payment|Customer]SheetViewModel to UpdatePaymentMethodInteractor. EditCardDetailsInteractor is handling CBC change now, that's why I added the function parameter
  *val onCardDetailsChanged: CardDetailsCallback is a callback that emits edit card parameters to a consumer (UpdatePaymentMethodInteractor)

I don't think onCardDetailsChanged fits into the viewAction because of the behaviour I described above. I could move the brandChanged function into a ViewAction interface. What do you think?

Is this unintentional? I see two callbacks for handling changes to card brand choice.

The one highlighted in this comments is to send an analytics event upstream. The other is for when the user selects a brand from the dropdown. I expanded more here

I could rename it to something with analytics in its name. What do you think?

This feels redundant since brand choice can be controlled from CardUpdateParams as well. Are we going to combine these later? Could each individual field be a view action?

This callback is for analytics. I don't want the component using this interactor to have to determine when the cardbrand is changed. CardUpdateParams has expiryDate and address values as well, so a new cardUpdateParam does not mean the card brand is changed. The consumer will need to add logic (trivial I admit) to determine if there's a new card brand.

internal data class CardUpdateParams(
    val expiryMonth: Int? = null,
    val expiryYear: Int? = null,
    val cardBrand: CardBrand? = null,
    val billingDetails: PaymentMethod.BillingDetails? = null,
)

Why does it default to CartesBancaires?

The CARD_WITH_NETWORKS test fixture defaults to CartesBancaires. It was probably an arbitrary choice when the fixture was created

private fun handler(
        card: PaymentMethod.Card = PaymentMethodFixtures.CARD_WITH_NETWORKS,
        cardBrandFilter: CardBrandFilter = DefaultCardBrandFilter,

usually tests follow a pattern of:

• setup
• test
• assert

This is also known as: arrange, act, assert

in this case I suppose the test should look like this. Note that there is a single line between each section which acts as a separator between each section: arrange, act and assert.

@Test
   fun  xxx() {
        var cardUpdateParams: CardUpdateParams? = null
        val handler = handler(
            onCardDetailsChanged = {
                cardUpdateParams = it
            }
        )

        handler.brandChanged(CardBrand.Visa)

        assertThat(cardUpdateParams).isEqualTo(cardUpdateParams(cardBrand = CardBrand.Visa))
   }

If we want to test initial states like defaulting to CardBrand.CartesBancaires or cardUpdateParams being null, we should keep that in a different test that is ONLY testing that.

Imho, these comments hurt more than the help -- imho, i'd remove them because the variable names seem to speak for themselves, but i'll leave it up to you guys.

imho, shouldShowCardBrandDropdown or isCardBrandDropdownShown are better names as this is a Boolean

s/BrandChoiceCallback/CardBrandChoiceCallback/
or maybe
s/BrandChoiceCallback/CardBrandCallback/

Calling this "CardDetailsCallback" introduces "Details" as new nomenclature. It seems like we avoid this by just changing the name of this to: CardUpdateParamsCallback

A lot of the exposed attributes here are implementation heavy and we should remove what's necessary. Ideally the interactor only exposes what is important to the UI which is:

• an observable state
• a set of view actions

The rest of these attributes should be implementation detail specific to the default implementation of the interactor. These can be managed as private parameters on the default implementation:

• The card being edited.
• The card brand filter.
• payment method icon
• analytics callbacks for card details changing and card brand choice.

See the SelectSavedPaymentMethodsInteractor as an example. The default create method exposed via the interactor can also just create the default implementation.

Let's use the interactor within CardDetailsUI and UpdatePaymentMethodInteractor in this PR.

There will be more view actions for handling other card edit changes? When we do add those actions, let's create a ViewAction type with all the possible actions that the interactor can handle then expose a single method called handleViewAction that the UI can use.

Doesn't need to happen in this PR!